Fix CI: eliminate duplicate runs and automate test DB schema#56
Conversation
- Scope push trigger to branches: [main] to prevent duplicate CI runs on push + pull_request for the same branch - Replace 350-line static SQL schema in testutil with actual migration runner, preventing test/production schema drift
There was a problem hiding this comment.
Pull request overview
This PR aims to stabilize CI by preventing duplicate workflow executions and ensuring backend tests always use the same database schema definition as production (via the real migration files).
Changes:
- Restrict CI
pushtrigger tomainto avoid double-running CI on feature branches (push + pull_request). - Replace the large in-test static SQL schema with a migration-based setup by calling
database.RunMigrations()after resetting the schema.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
backend/internal/testutil/database.go |
Resets the test DB schema and applies the real migration files instead of maintaining a hard-coded test schema. |
.github/workflows/ci.yml |
Scopes push CI runs to main to reduce duplicate workflow runs on PR branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| _, err := td.Exec(` | ||
| DROP SCHEMA public CASCADE; | ||
| CREATE SCHEMA public; | ||
| GRANT ALL ON SCHEMA public TO CURRENT_USER; | ||
| `) |
| // RunMigrations runs database migrations on test database | ||
| // migrationsPath returns the absolute path to the migrations directory | ||
| func migrationsPath() string { | ||
| _, filename, _, _ := runtime.Caller(0) |
Move test DB and LocalStack out of the main docker-compose.yml into a dedicated test compose file with ephemeral storage. Update run_tests.sh to use -p 1 to prevent parallel DB conflicts.
There was a problem hiding this comment.
Pull request overview
This PR aims to stabilize CI by preventing duplicate workflow runs on feature branches and by making backend tests use the real SQL migrations (instead of a hand-maintained schema) to avoid schema drift.
Changes:
- Scope GitHub Actions
pushtrigger tomainonly to avoid duplicate CI runs on PR branches. - Replace the large static test schema with a migration-based test DB setup (reset schema, then
database.RunMigrations()). - Introduce a dedicated
docker-compose.test.yml(test Postgres + LocalStack) and serializego testexecution in the local test runner.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-compose.yml | Removes init-time DB bootstrap/migrations and removes LocalStack from the default dev stack. |
| docker-compose.test.yml | Adds a separate compose stack for a tmpfs-backed test Postgres and LocalStack. |
| backend/run_tests.sh | Runs go test with -p 1 to reduce concurrency across packages. |
| backend/internal/testutil/database.go | Resets DB via schema drop + runs real migrations instead of static SQL. |
| .github/workflows/ci.yml | Restricts push workflow trigger to main only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| _, err := td.Exec(` | ||
| DROP SCHEMA public CASCADE; | ||
| CREATE SCHEMA public; | ||
| GRANT ALL ON SCHEMA public TO CURRENT_USER; |
| volumes: | ||
| db_data: | ||
| driver: local | ||
| localstack_data: | ||
| driver: local | ||
|
|
| ports: | ||
| - "5432:5432" | ||
| volumes: | ||
| - db_data:/var/lib/postgresql/data | ||
| - ./backend_go/migrations:/docker-entrypoint-initdb.d | ||
| - ./scripts/init-test-db.sh:/docker-entrypoint-initdb.d/00-init-test-db.sh | ||
| healthcheck: |
| POSTGRES_PASSWORD: test_password | ||
| POSTGRES_DB: ditto_test | ||
| ports: | ||
| - "5432:5432" |
Requests from www.jobditto.com to jobditto.com/api were blocked by CORS policy since they are different origins. Redirect www visitors to the canonical non-www domain instead of serving both.
- Use assert.Contains for scheduled_time since API returns full ISO format - Use NoteTypeReflection in SoftDelete test to avoid duplicate with earlier NoteTypeGeneral
…que constraint The test was sharing createdInterview with other tests, causing a duplicate key violation on the interview_notes unique type constraint.
There was a problem hiding this comment.
Pull request overview
This PR aims to stabilize CI by avoiding duplicate workflow runs and ensuring the test database schema is always derived from the real SQL migrations (preventing drift between test and production).
Changes:
- Scope GitHub Actions
pushtrigger tomainonly to prevent duplicate CI runs on feature branches. - Replace the large static test schema with
database.RunMigrations()after resetting the Postgres schema. - Add a dedicated
docker-compose.test.ymlfor ephemeral Postgres + LocalStack test dependencies, plus small test adjustments and config tweaks.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
docker-compose.yml |
Simplifies dev DB container setup by removing test DB init and init-time mounts. |
docker-compose.test.yml |
Adds a separate compose stack for test DB + LocalStack. |
backend/run_tests.sh |
Serializes Go tests (-p 1) to reduce DB-related test flakiness. |
backend/internal/testutil/database.go |
Resets schema by dropping public and runs real migrations via database.RunMigrations(). |
backend/internal/repository/interview_note_test.go |
Improves test isolation by using a dedicated interview for soft-delete tests. |
backend/internal/handlers/interview_test.go |
Makes scheduled time assertion tolerant of formatting differences. |
Caddyfile |
Adds www → apex redirect behavior. |
.github/workflows/ci.yml |
Limits push runs to main to eliminate duplicate CI runs per PR push. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| container_name: ditto-test-postgres | ||
| environment: | ||
| POSTGRES_USER: ditto_test_user | ||
| POSTGRES_PASSWORD: test_password | ||
| POSTGRES_DB: ditto_test | ||
| ports: | ||
| - "5432:5432" |
There was a problem hiding this comment.
This compose file binds Postgres to host port 5432 and sets a fixed container_name. That will conflict with the dev DB in docker-compose.yml (also 5432 / fixed name) if both are running, making it hard to run dev + tests concurrently. Consider removing container_name and/or mapping to a different host port (or rely on the Compose network and avoid host port publishing when not needed).
| container_name: ditto-test-postgres | |
| environment: | |
| POSTGRES_USER: ditto_test_user | |
| POSTGRES_PASSWORD: test_password | |
| POSTGRES_DB: ditto_test | |
| ports: | |
| - "5432:5432" | |
| environment: | |
| POSTGRES_USER: ditto_test_user | |
| POSTGRES_PASSWORD: test_password | |
| POSTGRES_DB: ditto_test | |
| ports: | |
| - "5433:5432" |
| POSTGRES_USER: ditto_user | ||
| POSTGRES_PASSWORD: ditto_password | ||
| POSTGRES_DB: ditto_dev | ||
| # Create test database on init | ||
| POSTGRES_MULTIPLE_DATABASES: ditto_dev,ditto_test | ||
| ports: | ||
| - "5432:5432" | ||
| volumes: | ||
| - db_data:/var/lib/postgresql/data | ||
| - ./backend_go/migrations:/docker-entrypoint-initdb.d | ||
| - ./scripts/init-test-db.sh:/docker-entrypoint-initdb.d/00-init-test-db.sh | ||
| healthcheck: |
There was a problem hiding this comment.
By removing the init-time test DB creation/migration mounts, this dev compose now only provisions ditto_dev. Since the repo’s test tooling expects a separate ditto_test DB/user, consider adding an inline comment here pointing developers to docker-compose.test.yml (or using Compose profiles) so running tests locally doesn’t become a silent setup footgun.
| @@ -1,4 +1,8 @@ | |||
| {$DOMAIN:localhost}, www.{$DOMAIN:localhost} { | |||
| www.{$DOMAIN:localhost} { | |||
| redir https://{$DOMAIN}{uri} permanent | |||
There was a problem hiding this comment.
The redirect target uses {$DOMAIN} without a default, while the site labels use {$DOMAIN:localhost}. If DOMAIN is unset, this will generate an invalid redirect URL (e.g., https://{uri}). Use the same default placeholder (or a request placeholder like {host}) in the redirect target to keep behavior consistent when DOMAIN isn’t provided.
| redir https://{$DOMAIN}{uri} permanent | |
| redir https://{$DOMAIN:localhost}{uri} permanent |
| _, err := td.Exec(` | ||
| DROP SCHEMA public CASCADE; | ||
| CREATE SCHEMA public; | ||
| GRANT ALL ON SCHEMA public TO CURRENT_USER; |
There was a problem hiding this comment.
RunMigrations drops the entire public schema, which will also drop extensions installed into public (common for pgcrypto). The SQL migrations rely on gen_random_uuid() defaults, but there is no CREATE EXTENSION in the migrations or codebase, so a fresh Postgres container is likely to fail during migration. Consider ensuring pgcrypto is installed after recreating the schema (or add an initial migration to create required extensions).
| GRANT ALL ON SCHEMA public TO CURRENT_USER; | |
| GRANT ALL ON SCHEMA public TO CURRENT_USER; | |
| CREATE EXTENSION IF NOT EXISTS pgcrypto; |
| // RunMigrations runs database migrations on test database | ||
| // migrationsPath returns the absolute path to the migrations directory | ||
| func migrationsPath() string { | ||
| _, filename, _, _ := runtime.Caller(0) |
There was a problem hiding this comment.
migrationsPath() ignores the boolean return from runtime.Caller. While it usually succeeds, failing to handle the ok flag can make debugging path issues harder in unusual build/test environments. Consider checking ok and either panicking with a clear message or returning a safe fallback.
| _, filename, _, _ := runtime.Caller(0) | |
| _, filename, _, ok := runtime.Caller(0) | |
| if !ok { | |
| panic("testutil: unable to determine caller for migrationsPath") | |
| } |
Summary
pushtrigger tobranches: [main]only — prevents duplicate CI runs (push + pull_request) on feature branchestestutil/database.gowith actual migration runner usingdatabase.RunMigrations()— prevents test/production schema drift that caused multiple CI failures in Epic 9Test plan